Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365

j-c-cook · 2022-08-07T22:07:52Z

A RotatingLogger object is created that constrains the file by file size, time or both. In the case of when both are constrained, the first that occurs will trigger the rollover.

Adds a deprecation warning to the TimedRotatingLogger. The TimedRotatingLogger now inherits from the generalized RotatingLogger.
When time is a constraint for the RotatingLogger, don't rollover if there have been no messages received since the last rollover
Update the doc string under RotatingLogger
Fix the tests to make them run with these changes

closes #1364

- Adds a deprecation warning to the TimedRotatingLogger. The TimedRotatingLogger now inherits from the generalized RotatingLogger.

j-c-cook · 2022-08-11T14:50:08Z

The BLFWriter will be able to be used like the following:

python -m can.logger -i socketcan -c vcan0 -b 250000 -f file.blf -t 600 --compression-level=1 --max-container-size=200000

Here, a rollover would occur every 5 seconds, or when the buffer and file size becomes 200000. Note that the default max_container_size is 128 * 1024.

Another option would be:

python -m can.logger -i socketcan -c vcan0 -b 250000 -f file.log -t 600 -s 100000

A rollover would occur every 5 seconds, or when the file size becomes 100000 bytes.

j-c-cook · 2022-08-11T21:12:43Z

When time is a constraint for the RotatingLogger, don't rollover if there have been no messages received since the last rollover

This is inherent to the implementation of logger. When a message is received then the logger is called. The file cannot rollover without there being a message passed to logger.

python-can/can/logger.py

Lines 246 to 251 in 88fc8e5

    
           try: 
        
               while True: 
        
                   msg = bus.recv(1) 
        
                   if msg is not None: 
        
                       logger(msg) 
        
           except KeyboardInterrupt:

can/__init__.py

codecov · 2022-09-29T14:51:08Z

Codecov Report

Merging #1365 (22bf537) into develop (4b1acde) will increase coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 22bf537 differs from pull request most recent head cabafb4. Consider uploading reports for the commit cabafb4 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1365      +/-   ##
===========================================
+ Coverage    65.16%   65.29%   +0.12%     
===========================================
  Files           81       81              
  Lines         8853     8779      -74     
===========================================
- Hits          5769     5732      -37     
+ Misses        3084     3047      -37

j-c-cook · 2022-09-29T15:19:43Z

@zariiii9003 This RotatingLogger class has been successfully running on multiple devices, that I manage, for about a month now (prior to the refactoring I've done today). There have been tens and possibly hundreds of hours of testing on the class by now. It is ready for a review. Please let me know what you think, thanks.

felixdivo

Regarding the discussion in #1364: I generally think that separating what can be split is a fair goal. In this case, however, I think having one class for this makes a lot of sense since it does simplify both user code and the implementation (I think having cooperating sub-classes to have a logger that supports both variants of rotations will not be easier to understand than this).

There are also a few nice small improvements in here that should be merged regardless of the new features!

We should finish the talk about the general API in #1364 before merging here.

can/logger.py

can/io/logger.py

Co-authored-by: Felix Divo <[email protected]>

@felixdivo

@felixdivo

Here is what was stated in the warning message in the `docs` workflow: Warning, treated as error: /home/runner/work/python-can/python-can/can/io/logger.py:docstring of can.io.logger.RotatingLogger:6:py:attr reference target not found: namer Error: Process completed with exit code 2.

Here's what the error check said: Run mypy --python-version 3.7 . can/io/logger.py:487: error: "__init__" of "RotatingLogger" gets multiple values for keyword argument "max_bytes" [misc] Found 1 error in 1 file (checked 55 source files) Error: Process completed with exit code 1.

See: hardbyte#1383

…c-cook/python-can into issue1364_TimedRotatingLogger

Generalize the RotatingLogger; size, time or both constraint

5c23248

- Adds a deprecation warning to the TimedRotatingLogger. The TimedRotatingLogger now inherits from the generalized RotatingLogger.

j-c-cook added 4 commits August 11, 2022 10:44

Merge branch 'develop' into issue1364_TimedRotatingLogger

7970ce1

Update test_log_virtual_sizedlogger to have a file path

019e41c

Update logger doc string

e091ba0

Move rollover time reset to capture when file_size triggers

5be3a81

Merge branch 'develop' into issue1364_TimedRotatingLogger

89bf921

j-c-cook commented Sep 29, 2022

View reviewed changes

can/__init__.py Show resolved Hide resolved

j-c-cook added 2 commits September 29, 2022 09:31

Change delta_t to max_seconds

0cf8383

Run logger test on RotatingLogger rather than SizedRotatingLogger

cd36995

j-c-cook added 3 commits September 29, 2022 09:57

Modify timeout period name and call Rotating if file_time

c9ba894

Add test for the timed logger given file_time input

dc75109

Update logger help statement

22bf537

j-c-cook marked this pull request as ready for review September 29, 2022 15:17

Merge branch 'develop' into issue1364_TimedRotatingLogger

83b901e

j-c-cook mentioned this pull request Dec 25, 2022

TimedRotatingLogger #1364

Open

Merge branch 'develop' into issue1364_TimedRotatingLogger

0a89a6c

felixdivo requested changes Dec 28, 2022

View reviewed changes

can/logger.py Outdated Show resolved Hide resolved

can/io/logger.py Show resolved Hide resolved

felixdivo assigned j-c-cook Dec 28, 2022

felixdivo added this to the Next Release milestone Dec 28, 2022

felixdivo added enhancement file-io about reading & writing to files labels Dec 28, 2022

j-c-cook and others added 5 commits December 28, 2022 10:25

Update can/logger.py

c3d8715

Co-authored-by: Felix Divo <[email protected]>

Adjust the example in docstring and update documentation

b6b75b9

@felixdivo

Update the deprecation warning message

a830c7a

Merge branch 'develop' into issue1364_TimedRotatingLogger

78f777c

j-c-cook and others added 10 commits December 28, 2022 11:49

Update API order to be like develop and add formats set

b8a7ec5

Include the previous changes made to _default_name

842dcbb

See: hardbyte#1383

Fix spacing in _default_name

2a466b9

Update logconvert to use the generalized RotatingLogger

49649e7

Fix comment to reference RotatingLogger

35d25c7

Format code with black

b763f4a

Add test for time rollover condition

29717f4

Merge branch 'issue1364_TimedRotatingLogger' of https://github.com/j-…

ad382b5

…c-cook/python-can into issue1364_TimedRotatingLogger

Add note to preserve size rotating test functions at v5.0

8aff574

j-c-cook requested a review from felixdivo December 28, 2022 19:49

Merge branch 'develop' into issue1364_TimedRotatingLogger

cabafb4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365

Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365

Uh oh!

j-c-cook commented Aug 7, 2022 •

edited

Loading

Uh oh!

j-c-cook commented Aug 11, 2022

Uh oh!

j-c-cook commented Aug 11, 2022 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Sep 29, 2022 •

edited

Loading

Uh oh!

j-c-cook commented Sep 29, 2022

Uh oh!

felixdivo left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365

Are you sure you want to change the base?

Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365

Uh oh!

Conversation

j-c-cook commented Aug 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

j-c-cook commented Aug 11, 2022

Uh oh!

j-c-cook commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

j-c-cook commented Sep 29, 2022

Uh oh!

felixdivo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

j-c-cook commented Aug 7, 2022 •

edited

Loading

j-c-cook commented Aug 11, 2022 •

edited

Loading

codecov bot commented Sep 29, 2022 •

edited

Loading